Bound DDS cubemap counts before face-count conversion - #745
Merged
Chuck Walbourn (walbourn) merged 2 commits intoSep 29, 2026
Merged
Chuck Walbourn (walbourn) merged 2 commits into
Chuck Walbourn (walbourn) merged 2 commits into
Conversation
| if (d3d10ext->miscFlag & DDS_RESOURCE_MISC_TEXTURECUBE) | ||
| { | ||
| // DDS_HEADER_DXT10.arraySize is a count of cubemaps; TexMetadata stores their faces. | ||
| if (metadata.arraySize > (SIZE_MAX / 6)) |
Collaborator
There was a problem hiding this comment.
We can safely make this bound:
if (metadata.arraySize > UINT16_MAX)
{
return HRESULT_E_ARITHMETIC_OVERFLOW;
}
Direct3D Hardware maximums are below this, and Direct3D12 actually uses a 16-bit uint for the DepthOrArraySize field.
Collaborator
Author
There was a problem hiding this comment.
Happy to use UINT16_MAX. Just confirming: do you intend that bound on the input cubemap count before multiplying by six, or on the resulting face count?
Collaborator
There was a problem hiding this comment.
Bounding it before the multiply solves the overflow problem. There's already a check below (applied unless using "ALLOW_LARGE_FILES") which will bound the final value to the actual hardware limits of Direct3D (which is 2048)
Roland Shum (ShumWengSang)
marked this pull request as ready for review
September 29, 2026 19:13
Chuck Walbourn (walbourn)
approved these changes
Sep 29, 2026
Chuck Walbourn (walbourn)
approved these changes
Sep 29, 2026
Chuck Walbourn (walbourn)
merged commit Sep 29, 2026
4c5123d
into
microsoft:main
113 of 119 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bound the DX10 cubemap count to
UINT16_MAXbefore multiplying it by six to obtain the face count. Counts above that limit returnHRESULT_E_ARITHMETIC_OVERFLOW.This follows the review clarification to bound the input cubemap count, not the resulting face count. The check remains after zero-count normalization and applies on both 32-bit and 64-bit targets, including when
DDS_FLAGS_ALLOW_LARGE_FILESis enabled. This deliberately replaces the earlier machine-width-dependent representability check with a stricter, architecture-independent input limit.The existing final hardware-limit checks remain unchanged: without the large-file option, the face count is still limited to 2048. With that option, an input count of 65535 remains acceptable to metadata parsing and yields 393210 faces; 65536 is rejected before multiplication. The accepted large boundary was tested through metadata parsing only, not image loading.
Validation
git diff --checkpass.Companion and limits
The maintained corpus fixture is proposed in walbourn/directxtextest#77. That PR replaces its earlier C++ helper with one small DDS file; this source PR contains no test-suite files.
The focused harness is local validation, not a full maintained-suite pass. Full-suite validation remains pending because the configured media corpus is unavailable locally. Historical validation of the earlier representability-only revision is not being presented as validation of this new policy. This is a correctness change, not a memory-safety or exploitability claim.